-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DateRangePicker] Migrate internal components to emotion #26326
Conversation
@mnajdova One thing that I wonder after did some of the migration for internal components. It seems like the second one is simpler and gives the same result as the first one? const Root = styled('div', {}, { skipSx: true })
const Root = styled('div') |
52e18b8
to
113ea0f
Compare
113ea0f
to
aeb397d
Compare
Argos fail due to |
@@ -1,7 +1,6 @@ | |||
import * as React from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include PickersCalendar
here because of style override (min-height
) in DateRangePickerViewDesktop
const weeksContainerHeight = (DAY_SIZE + DAY_MARGIN * 4) * 6; | ||
|
||
// TODO remove PickersCalendarClassKey in CalendarPickerSkeleton migration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cannot remove PickersCalendarClassKey and styles in this PR due to dependency to CalendarPickerSkeleton. will removed them in CalendarPickerSkeleton migration PR
What do you mean result as the first one? The default value for |
packages/material-ui-lab/src/DesktopDateRangePicker/DesktopDateRangePicker.tsx
Show resolved
Hide resolved
My bad, didn't explain it clearly. I mean why do we need to have |
We discussed it in one of the first pickers PR #25989 In my opinion we don't need to do it, but let's follow the convention it is at this moment, we can update before the beta version if we decide it's ok to have it on the internal components as well. |
These internal components follow the same approach for migration so I think it is easier to bundle them together to review.
PickersCalendar
DateRangePickerViewDesktop
DateRangePickerToolbar
DateRangePickerInput
I have followed (at least) the PR section of the contributing guide.